External memory management #270
Conversation
Changed version of node-addon-api on the package.json example see: [issue 206](#206 (comment))
…s and documentation
|
Hope to find time to review tomorrow. |
| The **Error** class is a representation of the JavaScript Error object that is thrown |
There was a problem hiding this comment.
Should this file be updated in this PR?
| Error handling represents one of the most important considerations when |
There was a problem hiding this comment.
Should this file b e updated in this PR?
doc/memory_management.md
Outdated
| allocated memory that is kept alive by JavaScript objects. | ||
|
|
||
| ```cpp | ||
| int64_t AdjustExternalMemory(napi_env env, int64_t change_in_bytes); |
There was a problem hiding this comment.
I think the env parameter should probably be a node-addon-api Env as opposed to a napi_env.
| @@ -0,0 +1,59 @@ | |||
| # RangeError | |||
There was a problem hiding this comment.
Don't think this file should be changed in this PR.
|
Generally looks ok, except there are files that I don't think should be changed in this PR. Otherwise only a few small comments. |
|
@mhdawson I changed the |
| @@ -3074,6 +3074,17 @@ inline Value EscapableHandleScope::Escape(napi_value escapee) { | |||
| return Value(_env, result); | |||
| } | |||
|
|
|||
| //////////////////////////////////////////////////////////////////////////////// | |||
There was a problem hiding this comment.
This seems to add just a method as opposed to a class with methods. I think in earlier discussion it was either going to be a MemoryManagement class (or may Util class) with the method to adjust the memory.
There was a problem hiding this comment.
It was my misunderstanding. Now I will provide to create a class that expose the method. Sorry.
|
This PR seems to change a lot of unrelated files. Can you fix up that up or open a new PR as an alternative? |
|
Hi @mhdawson please close this PR and later I will provide to open a new one. |
|
Closing and will look for new PR. |
Added utility function
AdjustExternalMemorylike discussed in the following issue:#260
I added the function
AdjustExternalMemorywith corresponding tests and documentation. Please give a review of my work.